psp-7209 do not display acquisition file until all loading state is c… #3580
Conversation
…omplete and context is initialized.
| if ( | ||
| loadingAcquisitionFile || | ||
| (loadingAcquisitionFileProperties && !isPropertySelector) || | ||
| file?.fileType !== FileTypes.Acquisition || |
There was a problem hiding this comment.
this is odd. Do you know when this would be true?
There was a problem hiding this comment.
Yes, so essentially there is a race condition where when the file is loading the previous file is still in context. The state transition to set loading to false occurs before the new file is set into the context, and so this occurs.
Another solution would be to set the file in the context to null immediately during load.
There was a problem hiding this comment.
This bug happens when you load a Research file into the sidebar (which populates the sidebar context), then without closing the sidebar go to an ACQ file (via the PIMS files tab of a property of the file). Then the container renders what is already in context thinking it is an ACQ file when it is not.
|
✅ No secrets were detected in the code. |
| if (!!file && file?.fileType !== FileTypes.Acquisition) { | ||
| throw Error('Context file is not an acquisition file'); | ||
| } |
There was a problem hiding this comment.
I actually think this is a good error check. Without it we would have never found this bug/race condition. Just my opinion, but I would actually like to see this restored and a similar check added to ResearchView - but we can discuss on our Friday meeting
There was a problem hiding this comment.
If we leave it in, it would be dead code, because the parent makes the same check. This if statement will never run with the recommended changes.
There was a problem hiding this comment.
well, in theory this view could be embedded "anywhere" by another container but as I said would like us to discuss this use case for other views that make use of a context that might not have the information the view is expecting
Codecov Report
@@ Coverage Diff @@
## dev #3580 +/- ##
==========================================
+ Coverage 69.57% 74.59% +5.02%
==========================================
Files 1371 898 -473
Lines 33859 18612 -15247
Branches 6290 5258 -1032
==========================================
- Hits 23557 13884 -9673
+ Misses 10049 4728 -5321
+ Partials 253 0 -253
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…omplete and context is initialized.